Skip to content

Conversation

@fluidnumerics-joe
Copy link
Contributor

Resolves #2383

The runtime was too short to reach the antimeridian

Note that this unit test now throws an error
```
parcels._core.statuscodes.GridSearchingError: Grid searching failed at (z=[0.], lat=[87.97271], lon=[181.24731])
```
@erikvansebille
Copy link
Member

Thanks for this patch, @fluidnumerics-joe; but it doesn't seem to work (yet). I've updated the runtime of the tests/test_advection.py.py:test_nemo_curvilinear_fieldset unit test so that it reaches the antemeridian; and it (still) throws an error. Do you know what's going on? Does this updated unit test help you isolate the issue?

@reint-fischer
Copy link
Contributor

Note that the error thrown before was at lon=180.2 which is larger than grid.lon.max=180, but having fixed this still leads to an error:

FAILED tests/test_advection.py::test_nemo_curvilinear_fieldset - parcels._core.statuscodes.GridSearchingError: Grid searching failed at (z=[0.], lat=[79.6822], lon=[179.7905])

@fluidnumerics-joe
Copy link
Contributor Author

The core assumption, noted in the comment of the changes, is that the particle position is less than 180, to match the way you were implementing periodic boundary conditions in the associated issue.

What is the assumed range of particle longitudes going forward in parcels?

@fluidnumerics-joe
Copy link
Contributor Author

This patch fixes the original issue reported where the particle longitude is -177.4

@reint-fischer
Copy link
Contributor

What is the assumed range of particle longitudes going forward in parcels?

@erikvansebille and I were discussing what the assumed range should be in parcels, and although (-180,180) seems logical to me, we also considered allowing particles to have any lon, which could be mapped to the same location using a modulo.

Although the patch is able to find the particle at lat=88, lon=-177.4, it still is not able to find a particle at lat=79.6822, lon=179.7905, which falls within np.max(fieldset.U.grid.lon) = np.float64(179.99891662597656).

@fluidnumerics-joe
Copy link
Contributor Author

Copy that. I'll get something more robust in place

@michaeldenes
Copy link
Member

What is the assumed range of particle longitudes going forward in parcels?

@erikvansebille and I were discussing what the assumed range should be in parcels, and although (-180,180) seems logical to me, we also considered allowing particles to have any lon, which could be mapped to the same location using a modulo.

Just a note, the ERA5 data when downloaded straight from CMS is in the range [0,360).

I'd vote for lon$\in \mathbb{R}$ and lat$\in [-90,90]$ if possible.

fluidnumerics-joe and others added 2 commits November 18, 2025 21:00
* Particle longitude and grid longitudes are re-mapped to [-180,180)
  inside the curvilinear particle in cell check method.
* Depending on the proximity of the particle to +/- 180, cell corners
  are adjusted to the image position that is closest to the particle.
By not requiring periodicBC kernel anymore
@erikvansebille
Copy link
Member

Thanks @fluidnumerics-joe, for fixing this bug! I just committed e24c631, which extends the unit test for curvilinear grids to much longer runtime (and thus multiple loops); this also required a small bugfix to the interpolator which makes it more robust now

Copy link
Member

@erikvansebille erikvansebille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy that with this PR, periodic boundaries on spherical grids should work out-of-the-box, without users having to worry about them

@fluidnumerics-joe fluidnumerics-joe merged commit 6fc7724 into v4-dev Nov 19, 2025
11 checks passed
@fluidnumerics-joe fluidnumerics-joe deleted the bugfix/2383-curvilinear-periodic-point-in-cell branch November 19, 2025 12:32
@github-project-automation github-project-automation bot moved this from Backlog to Done in Parcels development Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants